feat(Modal): add forceMount prop to allow modal to render in SSR#5659
feat(Modal): add forceMount prop to allow modal to render in SSR#5659maxarias-io wants to merge 7 commits intonuxt:v4from
Conversation
commit: |
|
woops let me check the types |
|
I ran into an issue while testing this locally, it seems that after you close the modal the body will still have Ideas? |
|
I'm not entirely sure the Dialog component is made for SSR, take a look at unovue/reka-ui#688 (comment) |
|
Just noting that it seems to be working on my end. The modal is rendered in SSR. Let me know if there's anything else I can do/test. |
📝 WalkthroughWalkthroughThis pull request introduces a structured portal configuration system to enable Server-Side Rendering (SSR) support for dialog components. A new Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes consist of highly homogeneous updates across 13 component files (each follows an identical +2/-1 pattern of importing 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/content/docs/2.components/modal.md`:
- Around line 363-391: The new "Force Mount" component-code block triggers
markdownlint MD007/MD018; scope a markdownlint disable to just this snippet by
wrapping the component-code block with per-snippet directives (e.g., add a short
markdownlint-disable/enable comment immediately before and after the
component-code block) so lint rules are suppressed only for this example; look
for the ":::component-code" snippet that contains the portal prop (portal: { to:
false, forceMount: true }) and apply the scoped disable/enable around it to fix
MD007/MD018 without affecting other docs.
In `@src/runtime/composables/usePortal.ts`:
- Around line 9-11: The type-guard isDialogPortalProps currently treats any
object with properties like "disabled" as DialogPortalProps, which misclassifies
HTMLElements (e.g., buttons); update isDialogPortalProps to first exclude DOM
nodes by checking that p is not an HTMLElement (or Node) instance before testing
for 'to', 'disabled', 'defer', or 'forceMount' properties so real DOM elements
are never considered DialogPortalProps; locate the isDialogPortalProps function
and add the HTMLElement/Node exclusion prior to the property-in checks.
- Around line 51-60: The returned computed object omits the defer flag from
DialogPortalProps, so when portal.value is a DialogPortalProps the defer
true/false is ignored; update the computed return (the function that currently
returns { to: to.value, disabled: disabled.value, forceMount: forceMount.value
}) to also include defer, e.g. compute defer the same way as forceMount by
checking portal via isDialogPortalProps(portal.value) and forwarding p.defer (or
false if absent), ensuring the new property name is defer and handled alongside
forceMount, to and disabled.
| ### Force Mount | ||
|
|
||
| Use the `portal` prop with an object to force the Modal content to render even when closed. This is useful for SSR when the modal should be visible on initial page load. | ||
|
|
||
| ::component-code | ||
| --- | ||
| prettier: true | ||
| ignore: | ||
| - title | ||
| props: | ||
| portal: | ||
| to: false | ||
| forceMount: true | ||
| title: 'Modal with force mount' | ||
| slots: | ||
| default: | | ||
|
|
||
| <UButton label="Open" color="neutral" variant="subtle" /> | ||
|
|
||
| body: | | ||
|
|
||
| <Placeholder class="h-48" /> | ||
| --- | ||
|
|
||
| :u-button{label="Open" color="neutral" variant="subtle"} | ||
|
|
||
| #body | ||
| :placeholder{class="h-48"} | ||
| :: |
There was a problem hiding this comment.
Fix markdownlint MD007/MD018 in the Force Mount example.
Line 371 (MD007) and Line 389 (MD018) are flagged in this new block. Consider scoping a markdownlint disable to this component-code snippet.
🛠️ Suggested fix
### Force Mount
+
+<!-- markdownlint-disable MD007 MD018 -->
::component-code
---
prettier: true
ignore:
- title
props:
portal:
to: false
forceMount: true
title: 'Modal with force mount'
@@
`#body`
:placeholder{class="h-48"}
::
+<!-- markdownlint-enable MD007 MD018 -->🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
371-371: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
389-389: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🤖 Prompt for AI Agents
In `@docs/content/docs/2.components/modal.md` around lines 363 - 391, The new
"Force Mount" component-code block triggers markdownlint MD007/MD018; scope a
markdownlint disable to just this snippet by wrapping the component-code block
with per-snippet directives (e.g., add a short markdownlint-disable/enable
comment immediately before and after the component-code block) so lint rules are
suppressed only for this example; look for the ":::component-code" snippet that
contains the portal prop (portal: { to: false, forceMount: true }) and apply the
scoped disable/enable around it to fix MD007/MD018 without affecting other docs.
| function isDialogPortalProps(p: unknown): p is DialogPortalProps { | ||
| return typeof p === 'object' && p !== null && ('to' in p || 'disabled' in p || 'defer' in p || 'forceMount' in p) | ||
| } |
There was a problem hiding this comment.
Guard against HTMLElements in isDialogPortalProps.
Line 9-11: any object with disabled (e.g., an HTMLButtonElement) is treated as DialogPortalProps, which can misclassify a portal target element and drop it (p.to becomes undefined). Consider excluding DOM nodes/HTMLElements before property checks.
🛠️ Suggested fix
function isDialogPortalProps(p: unknown): p is DialogPortalProps {
- return typeof p === 'object' && p !== null && ('to' in p || 'disabled' in p || 'defer' in p || 'forceMount' in p)
+ if (typeof p !== 'object' || p === null) return false
+ // Exclude DOM nodes/HTMLElements from being treated as portal config objects
+ if ('nodeType' in (p as any)) return false
+ return 'to' in p || 'disabled' in p || 'defer' in p || 'forceMount' in p
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function isDialogPortalProps(p: unknown): p is DialogPortalProps { | |
| return typeof p === 'object' && p !== null && ('to' in p || 'disabled' in p || 'defer' in p || 'forceMount' in p) | |
| } | |
| function isDialogPortalProps(p: unknown): p is DialogPortalProps { | |
| if (typeof p !== 'object' || p === null) return false | |
| // Exclude DOM nodes/HTMLElements from being treated as portal config objects | |
| if ('nodeType' in (p as any)) return false | |
| return 'to' in p || 'disabled' in p || 'defer' in p || 'forceMount' in p | |
| } |
🤖 Prompt for AI Agents
In `@src/runtime/composables/usePortal.ts` around lines 9 - 11, The type-guard
isDialogPortalProps currently treats any object with properties like "disabled"
as DialogPortalProps, which misclassifies HTMLElements (e.g., buttons); update
isDialogPortalProps to first exclude DOM nodes by checking that p is not an
HTMLElement (or Node) instance before testing for 'to', 'disabled', 'defer', or
'forceMount' properties so real DOM elements are never considered
DialogPortalProps; locate the isDialogPortalProps function and add the
HTMLElement/Node exclusion prior to the property-in checks.
| const forceMount = computed(() => { | ||
| const p = portal.value | ||
| return isDialogPortalProps(p) && p.forceMount === true | ||
| }) | ||
|
|
||
| return computed(() => ({ | ||
| to: to.value, | ||
| disabled: disabled.value | ||
| disabled: disabled.value, | ||
| forceMount: forceMount.value | ||
| })) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "usePortal.ts" -o -name "*Portal*" | head -20Repository: nuxt/ui
Length of output: 89
🏁 Script executed:
cat -n ./src/runtime/composables/usePortal.tsRepository: nuxt/ui
Length of output: 2141
🏁 Script executed:
rg "usePortal" -A 5 -B 2 --type ts | head -50Repository: nuxt/ui
Length of output: 588
🏁 Script executed:
rg "from.*usePortal|import.*usePortal" --type ts -A 3Repository: nuxt/ui
Length of output: 33
🏁 Script executed:
rg "usePortal" --type ts -B 2 -A 5Repository: nuxt/ui
Length of output: 588
🏁 Script executed:
fd -e ts -e tsx | xargs grep -l "usePortal" 2>/dev/null | head -10Repository: nuxt/ui
Length of output: 87
🏁 Script executed:
fd -type f -name "*Dialog*" -o -name "*dialog*" | grep -E "\.(ts|tsx|vue)$"Repository: nuxt/ui
Length of output: 221
🏁 Script executed:
fd "Dialog|dialog" -e ts -e tsx -e vueRepository: nuxt/ui
Length of output: 33
🏁 Script executed:
ls -la src/runtime/ && echo "---" && find src/runtime -maxdepth 2 -type dRepository: nuxt/ui
Length of output: 1121
🏁 Script executed:
ls src/runtime/components/ && echo "---" && find src/runtime/components -maxdepth 1 -type dRepository: nuxt/ui
Length of output: 1925
🏁 Script executed:
cat -n src/runtime/components/Modal.vue | head -100Repository: nuxt/ui
Length of output: 3880
🏁 Script executed:
cat -n src/runtime/components/Modal.vue | sed -n '100,200p'Repository: nuxt/ui
Length of output: 4653
🏁 Script executed:
cat -n src/runtime/components/Modal.vue | sed -n '200,250p'Repository: nuxt/ui
Length of output: 1270
🏁 Script executed:
grep -r "DialogPortal" --include="*.ts" --include="*.tsx" -A 5 -B 2 | head -50Repository: nuxt/ui
Length of output: 3151
🏁 Script executed:
cat package.json | grep -A 20 "dependencies"Repository: nuxt/ui
Length of output: 835
🏁 Script executed:
cat package.json | grep "reka-ui"Repository: nuxt/ui
Length of output: 74
Forward defer property from portal props to DialogPortal component.
Line 10 of the guard confirms defer is a valid property in DialogPortalProps, but lines 56-60 show it is not forwarded in the returned object. This causes portal: { defer: true } to be silently ignored. Since forceMount is forwarded, defer should be handled consistently:
Suggested fix
+ const defer = computed(() => {
+ const p = portal.value
+ return isDialogPortalProps(p) ? p.defer : undefined
+ })
return computed(() => ({
to: to.value,
disabled: disabled.value,
- forceMount: forceMount.value
+ forceMount: forceMount.value,
+ defer: defer.value
}))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const forceMount = computed(() => { | |
| const p = portal.value | |
| return isDialogPortalProps(p) && p.forceMount === true | |
| }) | |
| return computed(() => ({ | |
| to: to.value, | |
| disabled: disabled.value | |
| disabled: disabled.value, | |
| forceMount: forceMount.value | |
| })) | |
| const forceMount = computed(() => { | |
| const p = portal.value | |
| return isDialogPortalProps(p) && p.forceMount === true | |
| }) | |
| const defer = computed(() => { | |
| const p = portal.value | |
| return isDialogPortalProps(p) ? p.defer : undefined | |
| }) | |
| return computed(() => ({ | |
| to: to.value, | |
| disabled: disabled.value, | |
| forceMount: forceMount.value, | |
| defer: defer.value | |
| })) |
🤖 Prompt for AI Agents
In `@src/runtime/composables/usePortal.ts` around lines 51 - 60, The returned
computed object omits the defer flag from DialogPortalProps, so when
portal.value is a DialogPortalProps the defer true/false is ignored; update the
computed return (the function that currently returns { to: to.value, disabled:
disabled.value, forceMount: forceMount.value }) to also include defer, e.g.
compute defer the same way as forceMount by checking portal via
isDialogPortalProps(portal.value) and forwarding p.defer (or false if absent),
ensuring the new property name is defer and handled alongside forceMount, to and
disabled.
|
This should be implemented in Reka UI, closing in favor of unovue/reka-ui#2494. |
2 participants
🔗 Linked issue
Resolves: #3605
❓ Type of change
📚 Description
Reka UI's Teleport primitive (used by DialogPortal) conditionally renders content using:
<Teleport v-if="isMounted || forceMount">https://github.com/unovue/reka-ui/blob/v2/packages/core/src/Teleport/Teleport.vue#L41-L48
Without forceMount: The modal content is never rendered server-side because isMounted is false and forceMount defaults to false. Users see modals "pop in" after hydration.
With forceMount: true: The content renders during SSR regardless of mount state, enabling proper server-rendered modals for use cases like query-parameter-driven modal states.
This prop was available in Reka UI's DialogPortal but was not being forwarded by Nuxt UI's Modal component.
Also even if we disable the portal, the Teleport component will not render
📝 Checklist